-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🗑️ Add deprecation warnings to .new and #starttls #119
Draft
nevans
wants to merge
2
commits into
ruby:master
Choose a base branch
from
nevans:deprecations-for-new-and-starttls
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+146
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e479952
to
1319d2a
Compare
42a247d
to
688fae2
Compare
688fae2
to
031fb0f
Compare
cf830ef
to
6cea168
Compare
2141292
to
de2244e
Compare
de2244e
to
dfffcfe
Compare
e31b14e
to
782cea8
Compare
782cea8
to
56befa6
Compare
56befa6
to
0cd91b4
Compare
0cd91b4
to
06c3eba
Compare
06c3eba
to
272a3a5
Compare
272a3a5
to
3e070d8
Compare
* `ssl` was renamed to `tls` in most places, with backwards compatible aliases. Using `ssl` does not print any deprecation warnings. Using both `tls` and `ssl` keywords raises an ArgumentError. * Preparing for a (backwards-incompatible) secure-by-default configuration, `Net::IMAP.default_tls` will determine the value for `tls` when no explicit port or tls setting is provided. Using port 143 will be insecure by default. Using port 993 will be secure by default. Providing no explicit port will use `Net::IMAP.default_tls` with the appropriate port. And providing any other unknown port will use `default_tls` with a warning. 🚧 TODO: should we use a different config var for default tls params when port is 993 and `tls` is unspecified? 🚧 TODO: should we use a different config var for choosing `tls` when `port` is non-standard vs choosing `port` and `tls` when neither are specified? 🚧 TODO: should we use a different var for `default_tls` be used to config params when port is 993 but tls is implicit? Another var?
3e070d8
to
350e96b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Preparing for a (backwards-incompatible) secure-by-default configuration, Net::IMAP.default_ssl will be used when no explicit port or TLS setting is provided.
TODO: should truthy default_ssl be used to config params when port is 993 but ssl is implicit? Another var?
Moved all deprecated option handling to DeprecatedClientOptions, which is prepended to Net::IMAP.
Additionally, split
initialize
up into small helper methods making it easier to understand at a glance.This is a first step towards fixing #118.